Add EIP-1559 gas mode, nonce reservation, and exposed asset creation phases#269
Add EIP-1559 gas mode, nonce reservation, and exposed asset creation phases#269lupuszr merged 5 commits intov8/developfrom
Conversation
Lexpeartha
left a comment
Lexpeartha
left a comment
There was a problem hiding this comment.
Good separation of concerns I would say. Did not dive too deeply, but from my perspective I can read it easily and make sense out of it.
Also we should double check with tests if there was any accidental regression made, to me it looks alright but we should double check
| const requestedGasMode = | ||
| options.blockchain?.gasMode ?? | ||
| this.config.blockchain?.gasMode ?? | ||
| envGasMode ?? | ||
| DEFAULT_PARAMETERS.GAS_MODE; |
There was a problem hiding this comment.
I like the env setting, makes it easier for folks who are not super familiar with dkg.js
Though I think env should overwrite options and config then
| const supportsEip1559 = | ||
| feeHistory.supported && | ||
| feeHistory.baseFeePerGas?.length && | ||
| feeHistory.priorityFees?.length; |
There was a problem hiding this comment.
Did you check for neuroweb here? I'm not 100% sure but I remember baseFeePerGas and priorityFees were returned for it too even though it doesn't support the eip1559 smart gas pricing
There was a problem hiding this comment.
It just aggregates them together later to form the gasPrice
| ); | ||
| previousTxGasPrice = tx.gasPrice; | ||
| const nonce = await this.allocateNonce(blockchain); | ||
| previousTxGasPrice = tx.gasPrice ?? tx.maxFeePerGas; |
There was a problem hiding this comment.
with eip1559 we could probably get the real gasPrice after the tx is executed instead of using maxFeePerGas
| return blockchain?.publicKey; | ||
| } | ||
|
|
||
| async allocateNonce(blockchain) { |
There was a problem hiding this comment.
can you add some comments about this function?
4 participants
Body: